Rather than assigning the container's duration to the stream's duration, I think we should do something like:

if (!streamMetadata.durationSectonsFromHeader.has_value()) {
  return containerMetadata_.durationSecondsFromHeader;
}

I think that keeps it clear where we're getting our information from. So far, we've kept that explicit in our existing metadata fields.

One wrinkle here is that the metadata we report to users in Python is VideoStreamMetadata. That is, it's explicitly the stream metadata. We don't have a hierarchically clean way to add the container metadata there. So while it's kinda odd, maybe we should add a field for duration_seconds_from_container on the Python side.

To be clear, my current thinking is that:

1. In C++, we fall back to the container metadata where appropriate. This may be awkward because we don't pass around the container metadata, so we may want to add a durationSecondsFromContainer field to StreamMetadata.
2. In Python, we should add a field for duration_seconds_from_container to StreamMetadata, and we have logic to fall back to it in duration_seconds.

@NicolasHug, @Dan-Flores, I'm curious to hear your thoughts as well.

@scotts can you help me understand where you think we should add this?

if (!streamMetadata.durationSectonsFromHeader.has_value()) {
  return containerMetadata_.durationSecondsFromHeader;
}

I agree with your general point that we should avoid setting the stream metadata to the container metadata.

We might be able to avoid defining a new *_from_container attribute on the StreamMetadata: the ContainerMetadata metaclass exists in Python. We just don't expose it currently.

The original error happens here:

and this is a place where we do know the ContainerMetadata. Maybe we can just fallback to it in that function only (I haven't checked that this would solve the original issue). And, whether we want to expose ContainerMetadata is kind of a separate issue.

As a side note, reasoning about all these fallbacks hurts my brain. We have fallbacks in the Python StreamMetadata class, in the C++ initializeDecoder(), and in the C++ helpers like getNumFrames. It might be time we address our long-time issue of centralizing all of the fallback logic in C++ (#125 (comment))

@scotts @NicolasHug Thank you for the feedback!

I tried implementing fallback at line 413-418 in _video_decoder.py but I think that num_frames would be none. This is due to metadata.num_frames not having the container duration when it computes num_frames using duration_seconds_from_header when num_frames is missing from the video.